Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix FAD/Ubiquinone Duplicates #613

Merged
merged 11 commits into from
Jun 21, 2023
Merged

Fix FAD/Ubiquinone Duplicates #613

merged 11 commits into from
Jun 21, 2023

Conversation

Devlin-Moyer
Copy link
Collaborator

@Devlin-Moyer Devlin-Moyer commented Jun 7, 2023

Main improvements in this PR:

As discussed in #607:

  • Removes MAR02035 for being a duplicate of MAR03121
  • Removes MAR03211 for being a duplicate of MAR03212
  • Removes MAR03751 for being a duplicate of MAR03752
  • Removes MAR03783 for being a duplicate of MAR03784
  • Removes MAR04242 for being a duplicate of MAR04243
  • Removes MAR03769 for being a duplicate of MAR03770
  • Removes MAR03838 for being a duplicate of MAR08611
  • Removes MAR02366 for being a duplicate of MAR03136
  • Removes MAR02367 for being a duplicate of MAR03135
  • Removes MAR02370 for being a duplicate of MAR03128
  • Removes MAR02372 for being a duplicate of MAR03156
  • Removes MAR02369 for being a duplicate of MAR03149
  • Removes MAR02373 for being a duplicate of MAR03142
  • Removes MAR00483 for being a duplicate of MAR00482/MAR00449/MAR01169
  • Removes MAR08743 for being a duplicate of MAR04652

I hereby confirm that I have:

  • Tested my code on my own computer for running the model
  • Selected develop as a target branch
  • Any removed reactions and metabolites have been moved to the corresponding deprecated identifier lists

@Devlin-Moyer
Copy link
Collaborator Author

Devlin-Moyer commented Jun 7, 2023

I have no idea how to interpret the error message generated by the YAML validation check (I figured out the issue by manually inspecting each of the changes I made, which is not unreasonable, but it would've been nice if the error message had given me any hints about what I should've been looking for)

@feiranl
Copy link
Collaborator

feiranl commented Jun 7, 2023

I am a little bit afraid of this kind of changes. In case reviewers merge this PR, I tag this PR for draft first. Discussions will come later in the Issue page.

@feiranl feiranl marked this pull request as draft June 7, 2023 08:24
@Devlin-Moyer
Copy link
Collaborator Author

nope I still don't know why the YAML validation test is failing

@mihai-sysbio
Copy link
Member

I'll have a look, perhaps I can spot something

@mihai-sysbio
Copy link
Member

mihai-sysbio commented Jun 8, 2023

First thing I'm trying is to run the validation locally on this PR with

python code/test/sanityCheck.py

This returns the same error

Traceback (most recent call last):
File "/Human-GEM/code/test/sanityCheck.py", line 105, in
checkUnusedEntities(model, "genes")
File "/Human-GEM/code/test/sanityCheck.py", line 96, in checkUnusedEntities
assert len(unused_entities) == 0, f"Found unused {{entity_type}}!"
^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Found unused {entity_type}!

It seems the other PRs are passing this test, so a more in-depth look is required. Sorry I cannot do more at the moment, but I'll try to get back to this tomorrow.

@Devlin-Moyer
Copy link
Collaborator Author

Aha the problem is that MAR03769 was the only reaction that ENSG00000128928 (IVD) was associated with; it clearly should have also been associated with MAR03770, since IVD is an FAD-dependent enzyme, but removing MAR03769 made IVD into an "unused gene"
I'm gonna add ENSG00000128928 to the GPR of MAR03770 unless anyone has any objections to (hopefully) get this to pass the YAML validation test

@Devlin-Moyer
Copy link
Collaborator Author

since @feiranl requested it in #625 and seemed uneasy about this PR, I figured I'd also go through and merge the reaction annotations from the deleted reactions into the kept ones

@mihai-sysbio
Copy link
Member

@Devlin-Moyer I'm glad you figured out the unused gene.
I've pushed a commit on this branch that improves the validation output as suggested above. If you feel this is not the right place, please feel free to remove it from this PR.

@feiranl feiranl marked this pull request as ready for review June 16, 2023 08:47
@haowang-bioinfo
Copy link
Member

haowang-bioinfo commented Jun 19, 2023

@Devlin-Moyer please merge develop and incorporate latest changes to this branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants